All committers have signed the CLA.

The marshaled resources are actually fairly similar with the new library. The main differences come from inconsistencies in go-yaml/yaml.

For example, it would output:

    panels:
        - id: 4
          gridPos:
            h: 22
            w: 10
            x: 14
            "y": 0
          tags:
            - hosted-metrics

(note the inconsistent indentation: mostly 4 spaces but sometimes 2 spaces)

While goccy/go-yaml generates:

    panels:
        - id: 4
          gridPos:
              h: 22
              w: 10
              x: 14
              "y": 0
          tags:
              - hosted-metrics

(consistent, 4 spaces indentation)

Hi @K-Phoen,

I tried to grr pull and grr diff resources (i.e. dashboards) from a Grafana instance, and while I'd expect no differences, I see a few of them, that look like:

Dashboard.hOipiybnz changes detected:
--- Remote
+++ Local
@@ -292,7 +292,7 @@
                 language: plaintext
                 showLineNumbers: false
                 showMiniMap: false
-            content: |4-
+            content: |-
                 <h1 style="text-align: center;">Docker</h1>
                 <h3 style="text-align: center;"><a href="/d/MQ5mdQbnk/docker-monitoring">More</a></h1>
             mode: html

It also looks like similar differences are detected when following the same steps with the current version (using go-yaml/yaml). So, do you think these are acceptable? Or should these completely disappear with the lib replacement?

Other than that, is there any other test that you would suggest doing before merging this PR? (cc/ @malcolmholmes)
I'd be happy to help with getting this merged. So far, I've merged latest changes from master to solve conflicts.

Thanks!

The thing that stopped me merging this PR is the fact that it results in whitespace (and other) no-op diffs. Any users that are using diff to detect meaningful changes would find a whitespace change annoying.

And given that there's a simple solution (don't lead your queries with a newline), I didn't tackle the question of whether this was a valid/valuable change.

I'm totally open to using a different approach to YAML marshalling, I just want to know its enough to justify any discomfort users may experience.

The thing that stopped me merging this PR is the fact that it results in whitespace (and other) no-op diffs. Any users that are using diff to detect meaningful changes would find a whitespace change annoying.

And given that there's a simple solution (don't lead your queries with a newline), I didn't tackle the question of whether this was a valid/valuable change.

I'm totally open to using a different approach to YAML marshalling, I just want to know its enough to justify any discomfort users may experience.

What do you think would be actually annoying?

• grr pull + git status would result in a significant amount of changes
• grr diff would result in a significant amount of changes
• both

--

In my opinion (which might be totally wrong!), what I think that could be annoying is only the 2nd, cause would be equivalent to saying "your local files aren't in sync" - while this would not necessarily be true.

In that sense, I think we could kinda sort it with a different diff algorithm, that compares "objects" (like JSON comparison) not string literals. That should reduce most (if not all) the no-op diffs (it would still display white spaces and newlines within values, but I guess that should be fair cause they could technically have different meanings / end in different results.

Regarding the 1st one, I guess it's like when adding a new linter rule or something like that, could be a bit annoying as well, but less likely to cause problems if we provide a real comparator (grr diff tells the "truth").

Wdyt? cc/ @K-Phoen

Interesting questions - you're totally on the mark here. A JSON diff would be amazing, actually. And whitespace changes are less of a bad thing in PRs nowadays due to clever diff highlighting whitespace differently.

FTR: This PR improves matters but does not resolve them. Grafana dashboards can do some weird things with their JSON. Here's some issues that show up with this library when parsing and rendering dashboards found in the wild: goccy/go-yaml#418

converted to draft as there's nothing final here

Unfortunately, this library introduces a separate set of issues caused by the new library. I'm closing it for now. Should this library resolve its issues, we can re-open/reconsider. Keeping #245 open to keep track of the original issue.